-
Couldn't load subscription status.
- Fork 187
fix: move CSS imports from @Theme to bootstrap JS file #22518
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
flow-server/src/main/java/com/vaadin/flow/server/frontend/scanner/FrontendDependencies.java
Show resolved
Hide resolved
...pplication-theme/src/test/java/com/vaadin/flow/webcomponent/ApplicationThemeComponentIT.java
Outdated
Show resolved
Hide resolved
|
|
||
| protected <T> List<String> merge(Map<T, List<String>> css) { | ||
| protected <T> List<String> merge(Map<T, List<String>> outputFiles) { | ||
| // Ignore app shell imports and definitions for bundle build detection, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't app shell imports also be taken into account by flow as they are not added to the general imports anymore even though they were there before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from unit tests, the feature that is affected here is the bundle build.
Should the bundle include theme CSS imports? I don't know, but the current implementation didn't. So I assumed that my changes at some point produced a valid failure in the bundle ITs because of including CSS imports again. Ignoring here addresses this failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mshabarov what was the state for css in bundles.
From what I see in tests and code I would expect css to be bundles and Theme css especially
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Bundles were including the css, but stats.json wasn't indicating that, because it didn't consider the new file with their imports. Hope this change helps it: 9b23bc1
flow-server/src/main/java/com/vaadin/flow/server/frontend/AbstractUpdateImports.java
Show resolved
Hide resolved
|



Fixes #22517